Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Bijectors integration tests #353

Merged

Conversation

mhauru
Copy link
Contributor

@mhauru mhauru commented Nov 7, 2024

Copied over from EnzymeAD/Enzyme.jl#2037.

Closes #348

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

see 1 file with indirect coverage changes

Copy link
Member

@willtebbutt willtebbutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you for opening this.

Regarding the failing test: it's one that we know about already, and is related to this rather annoying upstream problem. We need some way of indicating that this is a problem. I propose the following:

  1. exclude the offending example from the main test suite
  2. call build_rule on the test case, which should be enough to trigger the error. Put this call inside a @test_broken thing, so that when we eventually fix the problem we find out about it.

Does this sound sensible to you?

test/integration_testing/bijectors/bijectors.jl Outdated Show resolved Hide resolved
@mhauru
Copy link
Contributor Author

mhauru commented Nov 7, 2024

I put in the following, does this do what you wanted?

        TestCase(
            function (x)
                layer = Bijectors.PlanarLayer(x[1:2], x[3:4], x[5:5])
                flow = Bijectors.transformed(
                    Bijectors.MvNormal(zeros(2), LinearAlgebra.I),
                    layer,
                )
                x = x[6:7]
                return Bijectors.logpdf(flow.dist, x) -
                       Bijectors.logabsdetjac(flow.transform, x)
            end,
            randn(Xoshiro(23), 7);
            name = "PlanarLayer7",
            # TODO(mhauru) Broken on v1.11 due to
            # https://github.com/compintell/Mooncake.jl/issues/319
            broken=(VERSION >= v"1.11"),
        ),
        if case.broken
            @test_broken begin
                test_rule(Xoshiro(123456), case.func, case.arg; is_primitive=false)
                true
            end
        else
            test_rule(Xoshiro(123456), case.func, case.arg; is_primitive=false)
        end

Copy link
Member

@willtebbutt willtebbutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect. Thank you!

@willtebbutt
Copy link
Member

I'll merge when CI has passed.

@willtebbutt willtebbutt merged commit 5a6afa1 into compintell:main Nov 7, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add integration test for Bijectors
2 participants